fix: refresh OAuth tokens with native fetch instead of a subprocess by griffinmartin · Pull Request #258 · griffinmartin/opencode-claude-auth

griffinmartin · 2026-07-29T02:52:03Z

Summary

refreshViaOAuth ran its HTTP request inside a child process spawned as process.execPath -e <script>. That assumes process.execPath is a JavaScript runtime. It isn't inside OpenCode — the plugin runs in a compiled single-file executable, so process.execPath is the OpenCode binary and -e is not a script to evaluate.

Captured from a real OpenCode runtime (v1.18.8, macOS):

{
  "execPath": "/opt/homebrew/Cellar/opencode/1.18.8/bin/opencode",
  "execPath_dash_e": { "threw": true, "status": 1, "stdout": "" },
  "globalFetch": "function"
}

opencode -e '<script>' prints the CLI help banner and exits 1 with empty stdout. stderr was discarded (stdio: ["pipe", "pipe", "ignore"]) and the failure was only log()ged — logging is off unless CLAUDE_AUTH_DEBUG is set — so every direct OAuth refresh failed silently and fell through to spawning the claude CLI. Trace of the production path before this change:

refresh_needed       source=Claude Code-credentials
refresh_started      source=oauth
refresh_failed       source=oauth  error="Command failed: /opt/homebrew/.../opencode -e ..."
refresh_fallback_cli
refresh_started      source=cli
refresh_success      source=cli          <- claude ran, but returned the SAME token

Node 18+ and Bun both expose a global fetch, so the subprocess is unnecessary. The refresh chain becomes async as a result.

The existing tests could not have caught this: node --test sets process.execPath to the node binary, where -e works fine.

What else is in here

Validating the fix against a live account surfaced four more defects, each with a failing-first test.

CLI fallback was burning real API requests. The claude CLI only rotates a token that is itself near expiry. Since #238 the sync tick calls refreshIfNeeded(undefined, 1 hour), so for the final hour of every token the plugin spent a real claude -p request every 5 minutes that returned the same token — then reported success, because the re-read credentials still passed expiresAt > now + 60s. That is exactly the idle token consumption #104 set out to eliminate; it was masked because the OAuth step it relied on never ran. The fallback is now scoped to the 60-second reactive window.

Concurrent refreshes raced. The sync timer calls refreshIfNeeded directly while the request path arrives via getCachedCredentials. Each rotation invalidates the refresh token it was issued against, so the two racing left one caller holding a dead token. A test for this failed with 2 concurrent OAuth calls before the guard was added.

Borrowed credentials were clobbering their lender. When a refresh exhausted, tryFallbackAccount handed back another account's tokens and refreshIfNeeded assigned them to target.credentials. Once those neared expiry the borrower exchanged the lender's refresh token and wrote the rotated result into its own store — destroying its credentials and rotating the lender's token out from under it. This was unreachable while refreshViaOAuth never succeeded; making it work exposed the path. Borrowed accounts are now tracked and routed through a path that re-reads their own source, refreshes only with their own token, and writes back only their own result.

The refresh had no retry. The token endpoint rate-limits valid refresh requests — observed as repeated HTTP 429s against a real account — and several OpenCode instances refreshing near expiry cluster their calls. The request path already retried 429/529, so fetchWithRetry moves to a shared http module and both paths use it instead of duplicating the logic. Its backoff also now honours the caller's AbortSignal; previously a retry-after could stretch refreshViaOAuth's 15s deadline to the 30s cap.

Plus a multi-process recovery: a rotation invalidates the refresh token every other instance holds, and the loser had no way to notice. Its own source is now re-read before the CLI fallback, so it adopts what the winner already wrote rather than spawning claude -p.

Related issues

Refs #104, #238.

Testing

make all — 249 tests pass, 0 lint warnings, clean build
Every fix here has a test that failed against the previous implementation for the right reason
Verified in a real OpenCode runtime that the request reaches the endpoint (80 ms, real HTTP status) instead of Command failed: opencode -e, and that refresh_cli_skipped fires where the old code would have spawned claude -p

Two test-infrastructure fixes came out of this. copySourceFiles did not stub node:child_process, so index.test.ts was invoking the real claude binary; and once the refresh moved to fetch, unstubbed credential tests were reaching the real token endpoint. Both now fail closed. Suite time dropped from 16.7s to ~11s.

Note on versioning

getCachedCredentials, refreshIfNeeded, refreshViaOAuth and forceRefreshActiveAccount now return promises. They are reachable from the package entry point, but internal to the plugin in practice, so this is deliberately not marked breaking and will cut a patch release.

Checklist

PR title follows Conventional Commits
make all passes locally (runs lint, build, and test)
Tests added or updated where applicable
README or docs updated where applicable

greptile-apps · 2026-07-29T02:59:22Z

Greptile Summary

This PR replaces the broken OAuth token refresh subprocess path (process.execPath -e <script>) with native fetch, which was silently failing in OpenCode because process.execPath points to the compiled binary rather than a JS runtime. It also scopes the claude CLI fallback to the 60-second reactive window (avoiding wasted API calls during the hourly proactive refresh), deduplicates concurrent refreshes with an in-flight map, and introduces a borrowedCredentialAccounts guard to prevent cross-account token clobber.

Core fix — refreshViaOAuth now uses globalThis.fetch with an AbortController timeout and the shared fetchWithRetry helper (moved from index.ts to the new src/http.ts), and is now async.
CLI fallback scoping — performRefresh only invokes the claude CLI when credentials are within the 60-second reactive window; the proactive 1-hour threshold now returns still-valid credentials without spawning a real API call.
Concurrent refresh deduplication — inFlightRefreshes collapses concurrent refreshIfNeeded calls for the same account into a single OAuth request, preventing multiple callers from racing and each receiving a now-dead token.
Borrowed credential guard — borrowedCredentialAccounts (WeakSet) prevents a recovered-then-re-expired borrowed account from exchanging or writing back the lender's refresh token.

Confidence Score: 5/5

Safe to merge. All changed code paths are well-exercised by 240 passing tests, including targeted regressions for the three failure modes (subprocess vs. fetch, early CLI spawn, concurrent refreshes).

The core rewrite (subprocess → fetch) is straightforward and the async propagation is consistent across every call site. The borrowed-credential guard handles every recovery branch: own-source recovery, own-token OAuth recovery, re-borrow, and exhaustion with correct state restoration. The in-flight deduplication is correctly keyed and scoped to one synchronous turn, so there is no window for a second caller to install a duplicate entry. No pre-existing behaviour is silently changed — the CLI fallback remains, just bounded to the window where it is actually effective.

Files Needing Attention: No files require special attention. The most complex logic is in src/credentials.ts (refreshBorrowedAccount and the inFlightRefreshes guard), and it is covered by dedicated tests for each branch.

Important Files Changed

Filename	Overview
src/credentials.ts	Core credential management rewritten as async; refreshViaOAuth replaced subprocess with fetch, in-flight deduplication added, CLI fallback correctly gated on 60s expiry window, and borrowed-credential guard implemented throughout.
src/http.ts	New module extracting fetchWithRetry (previously in index.ts) and adding sleepUnlessAborted so the AbortController timeout in refreshViaOAuth can interrupt retry backoff.
src/credentials.test.ts	Extensive new tests for fetch-based OAuth, concurrency deduplication, CLI fallback scoping, and borrowed-credential isolation; test harness made hermetic by stubbing child_process.
src/index.ts	All call sites of getCachedCredentials/refreshIfNeeded awaited correctly; sync timer callback made async; fetchWithRetry import moved from local definition to http.ts.
src/index.test.ts	Made hermetic by stubbing child_process in copySourceFiles; timer tick callbacks awaited; new test for fetchWithRetry abort-during-backoff added.
scripts/test-models.ts	getCachedCredentials call awaited to match the now-async signature.

Sequence Diagram

sequenceDiagram
    participant Timer as Sync Timer (5min)
    participant Request as API Request Path
    participant RIN as refreshIfNeeded
    participant IFM as inFlightRefreshes
    participant PR as performRefresh
    participant OAuth as refreshViaOAuth (fetch)
    participant CLI as claude CLI (execSync)
    participant KC as Keychain/Store

    Note over Timer,KC: Proactive refresh (1h threshold)
    Timer->>RIN: refreshIfNeeded(undefined, 3600s)
    RIN->>IFM: get(source) → null (no in-flight)
    RIN->>PR: performRefresh(target, creds)
    PR->>OAuth: refreshViaOAuth(refreshToken)
    OAuth->>KC: POST /v1/oauth/token (fetch + AbortController 15s)

    Note over Request,KC: Concurrent request during proactive refresh
    Request->>RIN: refreshIfNeeded(account, 60s)
    RIN->>RIN: "creds.expiresAt > now+60s → return creds early"

    OAuth-->>PR: new ClaudeCredentials
    PR->>KC: writeBackCredentials
    PR-->>RIN: oauthCreds
    RIN->>IFM: delete(source)
    RIN-->>Timer: fresh credentials

    Note over Timer,KC: OAuth fails within 60s reactive window
    PR->>KC: refreshAccount (check external rotation)
    KC-->>PR: null
    PR->>CLI: execSync(claude -p . --model haiku)
    CLI-->>PR: success
    PR->>KC: refreshAccount
    KC-->>PR: fresh credentials

    Note over Timer,KC: Borrowed account refresh
    PR->>PR: borrowedCredentialAccounts.has(target)
    PR->>KC: refreshAccount(target.source)
    KC-->>PR: own expired creds with refreshToken
    PR->>OAuth: refreshViaOAuth(own.refreshToken)
    OAuth-->>PR: new credentials for borrower
    PR->>KC: writeBackCredentials(target.source, ownCreds)

_{Reviews (6): Last reviewed commit: "test: pin borrowed state to the account ..." | Re-trigger Greptile}

Both paths could still present a lender's refresh token as the borrower's and persist the result to the borrower's store. refreshBorrowedAccount cleared the guard on the exhaustion path while target.credentials still held the lender's tokens, so the next cycle took the normal path and exchanged them. It now restores the account's own credentials when they are readable and otherwise keeps the guard. forceRefreshActiveAccount exchanged account.credentials.refreshToken with no borrowed check, so a forced refresh on a borrowed account rotated the lender's token. It now declines and leaves recovery to refreshIfNeeded. Reported by Greptile on #258.

fetchWithRetry slept between attempts with a bare setTimeout, so a caller that bounded its request with an AbortController got the retry delay (capped at 30s) as its effective timeout instead. refreshViaOAuth sets a 15s deadline, and a 429 carrying retry-after made it wait far past that, then spend two more attempts that could only fail. The backoff now resolves early on abort and returns the rate-limit response instead of retrying. Also documents that the post-loop request is reachable only when retries < 1, rather than leaving it looking like dead code. Reported by Greptile on #258.

refreshViaOAuth ran its request inside a child process spawned as `process.execPath -e <script>`, which assumes process.execPath is a JavaScript runtime. That does not hold inside OpenCode: the plugin runs in a compiled single-file executable, so process.execPath is the OpenCode binary and `-e` is not a script to evaluate. The call exited status 1 with empty stdout, stderr was discarded via stdio "ignore", and the error was only log()ged, so the direct OAuth refresh failed silently on every attempt and always fell through to spawning the claude CLI. Node 18+ and Bun both expose a global fetch, so the subprocess is unnecessary. The refresh chain becomes async as a result: getCachedCredentials, refreshIfNeeded, refreshViaOAuth and forceRefreshActiveAccount now return promises. These are reachable from the package entry point but are internal to the plugin in practice, so this ships as a patch rather than a major. This also scopes the CLI fallback to the reactive window. The claude CLI only rotates a token that is itself near expiry, so invoking it an hour ahead (as the proactive sync tick does) spent a real API request per tick and returned the same token, then reported success. It now runs only within 60 seconds of expiry and still-usable credentials are returned untouched. Concurrent refreshes of one account are collapsed into a single request. The sync timer calls refreshIfNeeded directly while the request path arrives through getCachedCredentials; because each rotation invalidates the refresh token it was issued against, the two racing left one caller holding an already-dead token. The existing tests could not catch this: `node --test` sets process.execPath to the node binary, where `-e` works.

refreshViaOAuth takes an optional timeoutMs so the abort path can be tested without waiting the full 15 seconds. Default behaviour is unchanged. Without it, a stalled response was accepted after 2s instead of surfacing as null. Also pins the deduplication behaviour when a refresh exhausts: callers that joined a failed attempt all observe null, and the retry round they trigger collapses into a single request rather than one per caller.

…very Three defects surfaced while validating the native-fetch refresh against a live account. Borrowed credentials were persisted onto the borrowing account. When a refresh exhausted, tryFallbackAccount handed back another account's tokens and refreshIfNeeded assigned them to target.credentials. Once those neared expiry the borrower exchanged the LENDER's refresh token and wrote the rotated result into its OWN store, destroying its credentials and rotating the lender's token out from under it. Borrowed accounts are now tracked and routed through a path that re-reads their own source, refreshes only with their own token, and writes back only their own result. This was previously unreachable because refreshViaOAuth never succeeded inside OpenCode; making it work exposed the path. The refresh had no retry. The token endpoint rate-limits valid refresh requests, observed as repeated HTTP 429s against a real account, and several OpenCode instances refreshing near expiry cluster their calls. The request path already retried 429/529, so fetchWithRetry moves to a shared http module and both paths use it rather than duplicating the logic. A rotation invalidates the refresh token every other instance holds, and the loser had no way to notice. Its own source is now re-read before the CLI fallback, so it adopts what the winner already wrote instead of spawning `claude -p`. Also fails the credentials test fetch closed. refreshViaOAuth uses the runtime's fetch, so unstubbed tests were reaching the real token endpoint.

Both paths could still present a lender's refresh token as the borrower's and persist the result to the borrower's store. refreshBorrowedAccount cleared the guard on the exhaustion path while target.credentials still held the lender's tokens, so the next cycle took the normal path and exchanged them. It now restores the account's own credentials when they are readable and otherwise keeps the guard. forceRefreshActiveAccount exchanged account.credentials.refreshToken with no borrowed check, so a forced refresh on a borrowed account rotated the lender's token. It now declines and leaves recovery to refreshIfNeeded. Reported by Greptile on #258.

fetchWithRetry slept between attempts with a bare setTimeout, so a caller that bounded its request with an AbortController got the retry delay (capped at 30s) as its effective timeout instead. refreshViaOAuth sets a 15s deadline, and a 429 carrying retry-after made it wait far past that, then spend two more attempts that could only fail. The backoff now resolves early on abort and returns the rate-limit response instead of retrying. Also documents that the post-loop request is reachable only when retries < 1, rather than leaving it looking like dead code. Reported by Greptile on #258.

refreshAccountsList swaps in accounts read fresh from their own stores, so the WeakSet entry goes with the object it was keyed on. That is correct only because the rebuilt account carries its own tokens rather than the lender's; this pins that invariant.

🤖 I have created a release *beep* *boop* --- ## [2.1.5](v2.1.4...v2.1.5) (2026-07-30) ### Bug Fixes * handle external rotation of the Claude Code credential ([#260](#260)) ([5a44883](5a44883)) * refresh OAuth tokens with native fetch instead of a subprocess ([#258](#258)) ([231165b](231165b)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

Brings in upstream v2.1.5 and v2.1.6: OAuth refresh over the runtime's native fetch instead of a spawned subprocess (griffinmartin#258), external Claude Code credential rotation handling with compare-and-swap writeback and a bounded 401 recovery loop (griffinmartin#260), thinking-block-safe tool-pair repair after compaction (griffinmartin#263), and transient-vs-terminal refresh classification with backoff plus a cross-process refresh lock (griffinmartin#264). Two fork patches are dropped because upstream now solves the same problems better. `resolveJsRuntime` and the OAuth subprocess envelope are gone: upstream griffinmartin#258 removes the subprocess entirely, so there is no `process.execPath` runtime to resolve and the whole failure mode that patch existed for no longer exists. The slow-path credential store re-read in `refreshIfNeeded` is gone too: upstream griffinmartin#260 re-reads every source unconditionally and guards the writeback with a compare-and-swap, which is strictly stronger than re-reading only before spending a refresh. Dead `clearCredentialCache` is removed in favor of upstream's `invalidateCredentialCache`. Five fork patches are retained. Billing identity stays `cli` rather than upstream's `sdk-cli`, in both the user-agent and the `CLAUDE_CODE_ENTRYPOINT` default, which is the reason this fork exists. Expiry timestamps are still floored to integers at the two sinks that write them (`syncToPath` and the auth loader callback), since credentials read straight from the keychain bypass `parseOAuthResponse`'s `Math.trunc`. SSE stream lifecycle instrumentation and the append-only debug log are re-applied on top of upstream's stream transform, which was unchanged by griffinmartin#263. The `claude` CLI is still resolved to an absolute path with stderr captured, because upstream reverted to a bare `execSync("claude ...")` that cannot find the binary under a launchd-managed server's minimal PATH. The child-process test harness is updated for the retained CLI patch: it now rewrites the two-symbol `node:child_process` import, treats a spawned CLI as succeeding rather than throwing (nothing calls `execFileSync` for OAuth anymore), counts POSIX `execFileSync` and Windows `execSync` spawns together via `__getCliSpawnCount`, and pins `CLAUDE_CLI_PATH` so binary resolution never probes the host filesystem. 337 tests pass, typecheck and lint are clean, and the built plugin loads. OpenCode session ID: ses_035a6417cffeukveuaZsQUEJFU

greptile-apps Bot reviewed Jul 29, 2026

View reviewed changes

Comment thread src/credentials.ts

Comment thread src/credentials.ts

greptile-apps Bot reviewed Jul 29, 2026

View reviewed changes

Comment thread src/credentials.ts

griffinmartin added 6 commits July 28, 2026 22:32

griffinmartin force-pushed the fix/oauth-refresh-native-fetch branch from 2611ad9 to c42baab Compare July 29, 2026 04:32

griffinmartin changed the title ~~fix!: refresh OAuth tokens with native fetch instead of a subprocess~~ fix: refresh OAuth tokens with native fetch instead of a subprocess Jul 29, 2026

griffinmartin merged commit 231165b into main Jul 29, 2026
7 checks passed

griffinmartin deleted the fix/oauth-refresh-native-fetch branch July 29, 2026 17:31

github-actions Bot mentioned this pull request Jul 29, 2026

chore(main): release 2.1.5 #259

Merged

Navigation Menu

fix: refresh OAuth tokens with native fetch instead of a subprocess - #258

fix: refresh OAuth tokens with native fetch instead of a subprocess#258
griffinmartin merged 6 commits into
mainfrom
fix/oauth-refresh-native-fetch

griffinmartin commented Jul 29, 2026 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

greptile-apps Bot commented Jul 29, 2026 •

edited

Loading

Important Files Changed

Sequence Diagram

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

griffinmartin commented Jul 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

What else is in here

Related issues

Testing

Note on versioning

Checklist

Uh oh!

Uh oh!

Uh oh!

greptile-apps Bot commented Jul 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Greptile Summary

Confidence Score: 5/5

Important Files Changed

Sequence Diagram

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

griffinmartin commented Jul 29, 2026 •

edited

Loading

greptile-apps Bot commented Jul 29, 2026 •

edited

Loading